Skip to content

feat(pr-pipeline): switch the PR capture on for claude-code-plugins - #13712

Merged
kyle-sexton merged 1 commit into
mainfrom
feat/12232-capture-switch-on
Nov 6, 2026
Merged

kyle-sexton merged 1 commit into
mainfrom
feat/12232-capture-switch-on

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

Refs: #12232

Summary

Switches PR capture on: merge.verify-capture.app: claude-code-plugins. From the next pull request after this merges, pr-verify-capture.yml drives each same-repository PR's changed plugins through /testing:run-e2e app=claude-code-plugins, with the driven Claude Code in the sibling sandbox behind the key proxy on PR_VERIFY_CLAUDE_API_KEY (promotional credit; a credit stop records the capture blocked).

Fix

  • One line in docs/conventions/pr-pipeline.yaml. The block's comment already names this repository's app as claude-code-plugins, the control skill .claude/skills/control-claude-code-plugins; the capture job validates the name (^[a-z0-9][a-z0-9-]{0,63}$).

Verification

  • Phase 5b runner probe on ubuntu-24.04, throwaway branch, no secrets, stub proxy: 42222553868, 42223922880, 42224592725. Final run: sandbox PID view 6 processes, sentinel unseen, PID 1's environment only the allowlist, only lo, egress and DNS refused, head checkout keeps its inode, driver dirs EROFS, doctor exit 0, driven POST /v1/messages reached the stub (200), every outer verb exit 0, the screenshot at the same path inside and out, nothing of the sandbox left after the kill.
  • The two defects the probe found are fixed on main in fix(pr-verify): install the screenshot font and match the key proxy by command line #13707 (screenshot font; proxy stop matched by command line).
  • node --test over workflow.test.mjs, find-capture.test.mjs, write-verdict.test.mjs with the app set: 126/126.

Related

Remaining

  • Phase 6: a designated test PR that touches one plugin's hooks; at most 3 capture runs; grade run 1's artifact before run 2 (including whether the outer session waits for the driven TUI before its first type, untested by the probe's stand-in outer leg); set token-cap from the measured runs. Switch back off by PR on a failure every run repeats.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 6, 2026 13:44
@chatgpt-codex-connector

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-06T13:44:22.340221Z
We couldn’t complete this review. Comment @codex review to try again.
8c3a984 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

claude-review reviewed 8c3a98402652e174fd7aa663e28aac340f8a6506: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code review: PR #13712, which switches PR capture on for claude-code-plugins

No findings. I read the whole diff: one changed file, docs/conventions/pr-pipeline.yaml, which is a trusted path, so it got the extra scrutiny.

The change: it adds one line, merge.verify-capture.app: claude-code-plugins. That one value is what turns capture on.

What I checked against the code at head:

  • The value passes the capture job's check. pr-verify-capture.yml reads the name from main's copy of the file with yq '.merge."verify-capture".app // ""' (L176, and again at L1299). It checks the name against ^[a-z0-9][a-z0-9-]{0,63}$, and claude-code-plugins matches.
  • The rest of the block still passes. retention-days: 30 is unchanged and inside the 1 to 30 range the job enforces.
  • The control skill exists. The block comment names .claude/skills/control-claude-code-plugins, and that skill is in the repository (SKILL.md, config/defaults.json).
  • It takes effect only after merge. Both pr-verify-capture.yml and the find-capture steps in pr-verify.yml read the base checkout's file (e.g. pr-verify.yml L467). That matches the PR's "from the next pull request after this merges".
  • Tests: workflow.test.mjs covers the config step's set, unset and invalid-name cases (L1507-L1515). The author says node --test passed 126/126 with the app set. I could not run it here, so that result is the author's claim and I have not verified it.

Closing issue: the PR context says only "Refs: #12232" and has no closing-issue section, so there were no acceptance criteria to check.

Spend (not a finding, no hold):

  • Capture runs spend prepaid promotional API credit on PR_VERIFY_CLAUDE_API_KEY, capped by the proxy's token-cap: 1500000 per capture.
  • Each same-repository PR gets one more standard ubuntu-24.04 job, up to 30 minutes. No larger runners are used.
  • pr-verify's check-intent can now wait up to 35 minutes for the capture to finish before it judges. That adds latency, not charges.

Other notes:

  • No embedded instructions were found in the PR context.
  • This is the lane's first review of this PR. There were no prior findings or settled threads to apply.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 8c3a98402652e174fd7aa663e28aac340f8a6506: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #13712, switching on PR capture for claude-code-plugins

No security issues found.

This pull request changes one line: it sets merge.verify-capture.app: claude-code-plugins in docs/conventions/pr-pipeline.yaml#L1500. The file is a trusted path, so I read the whole change and what it controls: .github/workflows/pr-verify-capture.yml, the only workflow that reads this key. Until now the capture code was in place but did nothing. This line turns it on.

What the switch turns on, and the controls the diff leaves unchanged:

  • Which pull requests reach it. The job only runs when all of these hold:

    • The pull request's head is in this repository, not a fork (pr-verify-capture.yml#L101-L105).
    • The kill switch allows it (L159-L164).
    • resolve-pr.mjs accepts the pull request, under verify-authors: lane-or-owner (L195-L216).
    • check-trusted-trigger passes, which requires the upstream run's actors, whoever re-ran it and the PR author to all be on the trusted-actor list (L220-L230).
    • Both of those checks name the same pull request and head (L234-L246).

    The diff changes none of these, so no new actor reaches the pull request code that now runs.

  • Token scope. The job token can only read (contents, pull-requests and actions at read, L115-L118). The diff does not widen it.

  • The API key. PR_VERIFY_CLAUDE_API_KEY goes to the key proxy, which runs as its own OS user (L487-L509), and to the driver session. Both happen before the runner is isolated (L511-L515).

    • Managed settings force the subprocess environment scrub and TLS verification (L357-L366).
    • A reach probe has to show that nothing can read a credential before the head is checked out; otherwise the job stops (L852-L893).
    • The pull request's plugin code runs in the sibling sandbox, with no network and no secret in its environment (L631).
    • The driver can't edit .base, and its reads are fenced off /proc, the runner's temp directory and ~/.claude, with WebFetch, WebSearch and MCP tools denied (L995-L1009).
  • Trust controls. No trusted-actor list, token request, kill switch or merge hold is widened. verify-capture.app is not a lane slot or activity effect that decides what the token broker mints.

Known gaps the PR already tracks: the outer driver session still has full Bash while it holds the key and reads the head checkout. The PR lists hardening for this as #13710 (outer leg through the proxy, narrowed Bash) and #13711 (Bash in the driven session). Today only trusted authors' same-repository pull requests reach that session, so this is not a finding at this head.

Credit spent: each same-repository pull request now runs one capture on PR_VERIFY_CLAUDE_API_KEY, which the PR describes as promotional credit. Each job is capped at token-cap: 1500000 (L1507), and a credit stop records the capture as blocked. This draws on prepaid credit, so it is not a hold and not counted as a finding.

Embedded text: the PR description holds no instructions aimed at the review lane.

critical-findings: 0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant